Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(esclient): default esclient.apiVersion to 7.x #126

Merged
merged 1 commit into from
Mar 26, 2020
Merged

fix(esclient): default esclient.apiVersion to 7.x #126

merged 1 commit into from
Mar 26, 2020

Conversation

missinglink
Copy link
Member

default esclient.apiVersion to 7.x

@missinglink
Copy link
Member Author

cc @orangejulius are we good to pull the trigger on this now the dust has settled?

@missinglink missinglink mentioned this pull request Mar 26, 2020
Open
@orangejulius
Copy link
Member

Interesting. I'm trying to wrap my head around what it means for the apiVersion to be 7.X instead of 7.6.

The elasticsearch-js docs aren't very clear, do you have any idea?

@missinglink
Copy link
Member Author

missinglink commented Mar 26, 2020 •
edited
Loading

My understanding is that 7.x is equivalent for 7.latest, unfortunately there is no 6.x 🤷‍♂

orangejulius
orangejulius approved these changes Mar 26, 2020
View reviewed changes
Copy link
Member

@orangejulius orangejulius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've gone through the last couple versions of the elasticsearch-js-legacy code, and looked at all the files that define API versions.

I think I've confirmed that the latest minor version of Elasticsearch, and the .x version always seem to track each other.

For example, when 7.4 was the latest version, 7.x was equivalent to setting 7.4. When 7.5 became the latest, 7.x became equivalent to 7.5.

I think that means 7.x is what we want to minimize breakage and maximize compatibility. So this is good to merge.

We'll probably want to bump the Docker images for all of the importers as well as the API after this change is merged and a new version of the pelias-config NPM module is released.

@missinglink
Copy link
Member Author

missinglink commented Mar 26, 2020 •
edited
Loading

Agreed this is probably not ideal but the minor version breaking change has forced our hand somewhat to act quickly to avoid widespread errors and bug reports.

Let's merge this and see what happens with ES fixing it, maybe we have better luck than last time 🤷‍♂

@missinglink missinglink merged commit db6b9af into master Mar 26, 2020
@missinglink missinglink deleted the elasticsearch_7_es_api branch March 26, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orangejulius orangejulius orangejulius approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@missinglink @orangejulius